Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallelism #370

Merged
merged 5 commits into from
Nov 14, 2024
Merged

Parallelism #370

merged 5 commits into from
Nov 14, 2024

Conversation

elijahbenizzy
Copy link
Contributor

@elijahbenizzy elijahbenizzy commented Sep 22, 2024

[Short description explaining the high-level reason for the pull request]

Changes

How I tested this

Notes

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

We are going to release this in beta mode. Remaining tasks, will turn into issues

  • Decide how to "label" the tasks
  • Add guard-rails for async to ensure people don't mess stuff up with the async generator

@elijahbenizzy elijahbenizzy force-pushed the parallelism branch 8 times, most recently from c4feb7b to 33ae0e7 Compare September 22, 2024 19:45
Copy link

github-actions bot commented Sep 22, 2024

A preview of ae1b728 is uploaded and can be seen here:

https://burr.dagworks.io/pull/370

Changes may take a few minutes to propagate. Since this is a preview of production, content with draft: true will not be rendered. The source is here: https://github.com/DAGWorks-Inc/burr/tree/gh-pages/pull/370/

@elijahbenizzy elijahbenizzy force-pushed the parallelism branch 11 times, most recently from 0ba0fca to 358d8d2 Compare September 25, 2024 04:58
Copy link
Collaborator

@zilto zilto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the code examples, it's very clear how this API provides value. The introduction should ground and illustrate the concepts in more concrete terms.

docs/concepts/parallelism.rst Outdated Show resolved Hide resolved
Parallelism
===========

Burr allows for sets of actions/subgraphs to run in parallel. In this section we will go over the use-cases/how to run them!
Copy link
Collaborator

@zilto zilto Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify by avoiding the / character and being more descriptive. Unclear if each "parallel branch" must have the same action/subgraph.

Rephrased:

Burr can run multiple actions in parallel. Each parallel branch can contain one or more actions, and different branches can have different actions. This is useful when:
(simple use case)

  • Trying different prompts with an LLM
  • Trying a prompt with different LLMs
  • Trying multiple prompts with multiple LLMs
  • Do semantic search and web search simultaneously for information retrieval
  • and more! Just like Burr in general, these concepts are generic and can be applied to non-LLM applications

This section shows how to enable parallelism and presents use cases.

It's important to paint a picture about "what can it do" in the first sentence because reading the TL;DR wasn't super helpful for me, then we jump into the API and I still can't picture it.

Currently, this top-level intro, the TL;DR and the Overview section are a bit redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks

Burr allows for sets of actions/subgraphs to run in parallel. In this section we will go over the use-cases/how to run them!

General Idea/TL;DR
==================
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's the right header hierarchy. I'd remove the General Idea/TL;DR and promote everything underneath it

image

TL;DR
-----

Burr enables graph-level parallelism by having a "parallel action" that delegates to multiple sub-actions/graphs that run in parallel.
Copy link
Collaborator

@zilto zilto Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being the first header, this is difficult to decrypt. It's unclear if "parallel action" is a Burr construct. If yes, then simply name the Python objects. Even though there's a navigation menu, I'd use this section to give details on all the different approaches.

Rephrased based on the keypoints I identified:

Burr provides a high-level and a low-level API for parallelism. The high-level API supports many different patterns and should be sufficient for most use cases.
High-Level

  • MapStates: Apply an action to multiple values in state then reduce the action results (e.g., different prompts to the same LLM).
  • MapActions: Apply different actions to the same state value then reduce the actions result (e.g., same prompt to different LLMs).
  • MapActionsAndStates: Do the full cartesian product of actions and state values (e.g., try different prompts with multiple LLMs)
  • RunnableGraph: Combined with the above options, you can replace a single Action by a Graph composed of multiple actions.

With the low-level API, you can manually determine how parallel actions or subgraphs are executed.

Run the same action over different states
-----------------------------------------

For case (1) (mapping states over the same action) you implement the `MapStates` class that provides the following:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of the abstract concepts, refer to the example's content.

  • We define a regular action query_llm_action() using the @action decorator. We also create a subclass of MapStates named TestMultiplePrompts, which must implement .reads(), .writes(), .action(), .states(), and .reduce().
  • .reads() / .writes() define the state value it can interact with, just like the @action decorator
  • .action() leverages the query_llm_action() previously defined
  • .states() can read value from State and yields values to pass to the . action(). In this case, it updates the prompt state value that's read by query_llm_action(). (the example hardcoded a list of prompts for simplicity, but this would be read from state)
  • .reduce() receives multiple states, one per .action() call, where the llm_output value is set by query_llm_action() in .action(). Then, it must set all_llm_output as specified in the MapStates.writes() method.

I understand why the hardcode values are present in the example, but it made me scratch my head for a second. The above reference should be pretty easy to copy-paste for the subsequent sections

docs/concepts/parallelism.rst Outdated Show resolved Hide resolved
docs/concepts/parallelism.rst Show resolved Hide resolved
docs/concepts/parallelism.rst Outdated Show resolved Hide resolved
docs/concepts/parallelism.rst Outdated Show resolved Hide resolved
Lower-level API
---------------

The above compile into a set of "tasks" -- sub-applications to run. If, however, you want to have more control, you
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Briefly name the Python constructs involved and how they differ from the "high-level" API

Idea

All of the aforementioned high-level API are implemented as subclasses of TaskBasedParallelAction. You can subclass it directly and implement the .tasks() method that yields SubGraphTask, which can be actions or subgraphs. These tasks are then executed by the burr.Executor implementations

@elijahbenizzy elijahbenizzy force-pushed the parallelism branch 4 times, most recently from ccf8ff1 to 45497c8 Compare September 25, 2024 23:04
@skrawcz
Copy link
Contributor

skrawcz commented Sep 30, 2024

question - does this support async paths?

@elijahbenizzy
Copy link
Contributor Author

question - does this support async paths?

Yes see the points on async executor. Not fleshed out in this doc but it will be included in the first release

Copy link

@yiweig yiweig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First off, thanks so much for burr! We've been using it to build some agentic workflows at my job and it's been a breeze to set up and iterate.

I was directed to this PR from #339, and briefly looked over the new API. I love the direction you guys are going with this!

Had a few questions related to mapping the same state over different actions because that's a use case we're interested in, let me know if any of my questions don't make sense or if you need me to clarify!

from typing import Callable, Generator, List

@action(reads=["prompt", "model"], writes=["llm_output"])
def query_llm(state: State, model: str) -> State:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be named query_llm_action? The first example and the text above use that name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good point re: consistency! I think it's better if everything else is called query_llm -- this is the only one with action appended, and the docs switch between. Changed!

Comment on lines +157 to +164
query_llm.bind(model="gpt-4").with_name("gpt_4_answer"),
query_llm.bind(model="o1").with_name("o1_answer"),
query_llm.bind(model="claude").with_name("claude_answer"),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The examples here use the same action with different parameters bound to it. Is it possible to instead map over different actions completely? For example something like this:

@action(reads["query"], writes=["llm_output"])
def query_llm(state: State) -> State:
    return state.update(llm_output=some_function(state["query"]))

@action(reads["query"], writes=["llm_output"])
def count_tokens(state: State) -> State:
    return state.update(llm_output=another_function(state["query"]))

@action(reads["query"], writes=["llm_output"])
def draw_chart(state: State) -> State:
    return state.update(llm_output=a_third_function(state["query"]))

def actions(self, state: State) -> Generator[Action | Callable | RunnableGraph, None, None]:
    for action in [
        query_llm,
        count_tokens,
        draw_chart,
    ]:
        yield action

As a follow up question, is there a requirement that all the actions have to write the same keys? My naive intuition is that this is not a requirement, but I would have to do some extra work in the reduce function to make sure I get all available outputs:

@action(reads["query"], writes=["llm_output"])
def query_llm(state: State) -> State:
    ..
@action(reads["query"], writes=["token_count"])
def count_tokens(state: State) -> State:
    ..
@action(reads["query"], writes=["chart_bytes"])
def draw_chart(state: State) -> State:
    ..

def reduce(self, states: Generator[State, None, None]) -> State:
    outputs = {}
    for state in states:
        if "llm_output" in state:
            outputs["llm_output"] = state["llm_output"]
        if "token_count" in state:
            outputs["token_count"] = state["token_count"]
        if "chart_bytes" in state:
            outputs["chart_bytes"] = state["chart_bytes"]
        return state.update(outputs)

def writes() -> List[str]:
    return ["llm_output", "token_count", "chart_bytes"]

Alternatively, perhaps it would be better to do this to avoid burr complaining that a particular state key was not present in the output

def reduce(self, ..):
    outputs = {}
    ..
    return state.update({"outputs": outputs})

def writes():
    return ["outputs"]

Sorry for the long comment, just trying to wrap my head around the possibilities with the new API!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Yes, I think that's supported (If I understand correctly). It's a little nuanced (and I could explain it better), but the idea is that the generator can yield an Action object, a Callable (function that will get turned into an action, or a RunnableGraph (a subgraph that functions as an action). Specifically, this allows you to do exactly what you want/mix + match.

The way to reconcile this with the example (with multiple .bind()) calls is that the .bind creates a new action, so for the sake of the actions generator it doesn't really matter whether it was created through .bind(...) or created through specifying another action entirely!

Regarding the follow-up, currently, yes, it's necessary to apply defaults. This is not a feature of parallelism, rather a feature of Burr. Specifically, we want some strict sense of what an action writes so we have guarentees on what should show up next. The general approach is to apply defaults -- E.G. say if state doesn't contain foo, it'll be None -- I think the simplest in your case would be to do something like:

def reduce(self, states: Generator[State, None, None]) -> State:
    outputs = {'llm_output' : None, 'token_count' : None, 'chart_bytes' : None}  # initialize so you can overwrite with results
    # This is generic, or just have it like you did above, easier to read maybe
    for state in states:
        for key in list(outputs):
            if key in state:
                output[key] = state[key]
        return state.update(outputs)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, thank you! This new API is going to be very helpful for us :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Would love to hear about how you'd be using it/specifics (only if you feel comfortable sharing). This can help us get a sense of ergonomics (and we can send you test versions if you're interested in messing with it). Feel free to join the discord and DM us if that would be useful!

Comment on lines 160 to 163
]
yield action

def state(self, state: State) -> State::
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think there's a colon : missing from the for and an extra colon at the end of the def state(..) :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Good find. Will update 🙏

We were missing the annotations column and the width was wrong, leaving
an unshaded area.
Copy link
Contributor

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's get v1 out

Comment on lines 451 to 454
- ``burr.parallelism.MultiThreadedExecutor`` -- runs the tasks in parallel using threads (default)
- ``burr.parallelism.MultiProcessExecutor`` -- runs the tasks in parallel using processes
- ``burr.parallelism.RayExecutor`` -- runs the tasks in parallel using `Ray <https://docs.ray.io/en/latest/index.html>`_
- ``burr.parallelism.Dask`` -- runs the tasks in parallel using `Dask <https://dask.org/>`_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this section should be updated to reflect what we've shipped though?

@elijahbenizzy elijahbenizzy merged commit 1272c42 into main Nov 14, 2024
12 checks passed
@elijahbenizzy elijahbenizzy deleted the parallelism branch November 14, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants